-
Notifications
You must be signed in to change notification settings - Fork 464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unit test reactivation #780
base: master
Are you sure you want to change the base?
Conversation
create a static library of all apmplanner classes except of main, moved qgroundcontrol.pro to apmplanner_core.pro to keep the build process the same as before
removed old project configuration and source files from qgcunittest.pro because that is now part of the new common apmplanner-core library added a test for the qt test framwork to make sure the new setup works as expected
versions, copied from ApmFirmwareConfig.cc added unit tests for version comparator
in ApmFirmwareConfig.cc This bug was already fixed in AutoUpdateCheck.cc commit 8de0f15
… defined in the regex, therefore it is always 2 and the if statement is useless
…rsion string in one place fixed uninitialized variable warning
…s per group, allow muliple digits in build number
Thanks, Nice refactor of the compare version functions (and it should only have been in one place) My only question is that the old regexp allowed for 1.2, 1.3, 1.4 etc optional build. with optional rc-X tag. I'm not sure what happens with lines at https://github.com/mrpilot2/apm_planner/commit/81b1762d4c55a69badb9d1a5f12163eeb6257f72#diff-e67de0f4b1bb740829b564491a77041fR38 I'll merge the code changes in, hopefully tomorrow. And now you have readied the test framework, we can write the extra test cases ;) just quickly that would be 1.0 -> 1.1 = true Thx 👍 |
…d number is present
Hi, The capture groups for build and rc-X tag are also optional, as it was in the old regex. The function capture() always returns the number of capture groups defined in the regex. in our case captureCount() is always 6 no matter if optional groups are found or not. Optional capture groups may result in an empty string. The regex now has ensured that the capture string contains a number or is an empty string. If it is an empty string the conversion toInt() will "fail", which results to a 0. Summary: if you call, for example. .cap(3).toInt on an optional entity, the outcome is 0 (integer) I have added the tests you have suggested to prove this behaviour. |
Thanks for the PR. I have started the merge but have been slowed by a build issue on OS X . Should get the fix in at the weekend. 👍 |
Sorry, I haven't forgotten about this PR, just swamped with other work |
@mrpilot2 if you like to rebase these changes on master, I can merge the PR, or I can do it (but it might take little longer) I cleaned up most of my other PRs, I'm going to see if I merge all of them in the coming weeks |
Hi,
I have reactivated unit tests for the project.
My intention was to create a common library that is used by the application and the unit tests,
but keep the build process the same as before.
Therefore I performed the following steps:
These steps keep the build process the same as before, that means if no special config is added to qmake, the main application gets built.
If CONFIG+=test is added to qmake the unit tests will be built instead of the main application. The resulting executable is named apmplanner2_test.
To test the new unit test setup I have extracted the function that compares version strings from ApmFirmwareConfig and AutoUpdateCheck and implemented a new class VersionComparator and added tests for this class.